Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[for discussion] Draft proposal - arbitrary expressions for style functions #4715

Closed
wants to merge 18 commits into from

Conversation

anandthakker
Copy link
Contributor

@anandthakker anandthakker commented May 16, 2017

Starting work here on a draft/strawman implementation for adding arbitrary expressions to style functions. This first attempt attempts to synthesize the fruits of a few long-running discussions on the topic, in particular, mapbox/mapbox-gl-function#28, #4077, and #4079.

Important use cases that the final design should cover:

  • unit conversion
  • number formatting
    • decimal precision
    • thousands separator
  • conditionals (for the token defaults use case)
  • concatenation (for the token defaults use case)
  • customizable interpolation
  • arithmetic on feature properties within functions
  • localization

Expression-based style functions

Proposed plan:

  • deprecate zoom+property functions
  • deprecate property functions
  • keep stop-based zoom functions
  • allow data-driven expressions (a) as style property values, and (b) as output values for zoom function stops. (I.e., (a) replaces "property functions", and (b) replaces zoom+property functions.)

See docs/style-spec/expressions.md for the spec details.

See debug/expressions.html for a couple example usages.

Things we should design into the initial implementation

  • a way to map exact values to subexpressions, replacing type: categorical currently proposed as match
  • a way to match a value to a corresponding interval, replacing type: interval currently proposed as a combination of switch and between
  • a way to easily interpolate values linearly
  • number to string with specified number of decimal places (1.2345 to "1.23") possible with round

Things we could add support for later

  • a way to easily interpolate values exponentially, replacing type: exponential
  • number to localized string (21000 to "21,000" or "21 000" depending on locale)
  • methods for converting between distance units (metres, feet, kilometres, miles)
  • some way of localizing text by choosing the right name property
  • time formatting/localization? temperature? money?

Questions/issues

  • Should we cover image fallbacks as part of expression evaluation, or separately?

Additional ideas

  • Using something like http://jsep.from.so/ ( h/t @1ec5 ), it should be quite straightforward to parse expression text into the AST structure outlined above.
    • This may be useful both for the studio interface (if that's the route @mapbox/studio wants to go) and as a convenience alternative in the GL JS API (i.e. allowing something like circle-radius: { type: "expression", expression: "(1 + properties.foo) * 2^map.zoom)" } when specifying styles via the runtime api.)

cc @mapbox/gl

@ansis
Copy link
Contributor

ansis commented May 17, 2017

Great!

When an expression-based style function is both zoom-and-feature-dependent, we must use an approximation

I don't think we should support map properties (zoom, pitch, rotation) in expressions. I think the approximations would be rough enough that they'll produce confusing behavior. I'm not sure the rough edges and added complexity are worth it here.

I think that allowing expressions as the output values within all types of functions should provide enough flexibility for combining zoom + expressions.

@kkaefer
Copy link
Member

kkaefer commented May 17, 2017

We should also add e to the constant list. Having some sort of call syntax for complex function calls like for #4136, lowercasing/uppercasing, and formatting might be useful as well.

@kkaefer
Copy link
Member

kkaefer commented May 17, 2017

It looks like this draft is synthesizing JavaScript functions from the stylesheet input. While this works for a mock implementation, I think we should go with an actual interpreter since it'll allow us to validate input much better, and we'll have to do it this way anyway in Native. Additionally, creating Function objects in JS will make it harder to pass through CSP (#559).

@anandthakker
Copy link
Contributor Author

lowercasing/uppercasing, and formatting might be useful as well

@kkaefer for "formatting", are you talking about printf-like formatting? or something else?

@kkaefer
Copy link
Member

kkaefer commented May 17, 2017

Yeah, mostly number formatting (thousands separator, fractional separator, rounding/precision). String formatting with placeholders might be useful too.

@anandthakker
Copy link
Contributor Author

I don't think we should support map properties (zoom, pitch, rotation) in expressions. I think the approximations would be rough enough that they'll produce confusing behavior. I'm not sure the rough edges and added complexity are worth it here.

I'm not convinced that the approximations we're already using for stop-based composite functions are really any less confusing. Sure, GL JS manages to send four stops into the shader, but in native, we only send two. See also mapbox/mapbox-gl-native#8460.

That said, I'm ambivalent between the current proposal and something like:

I think that allowing expressions as the output values within all types of functions should provide enough flexibility for combining zoom + expressions.

Although I do think that the possibility of zoom-and-property stops and expressions would be pretty weird and confusing.

@anandthakker
Copy link
Contributor Author

I think we should go with an actual interpreter since it'll allow us to validate input much better, and we'll have to do it this way anyway in Native. Additionally, creating Function objects in JS will make it harder to pass through CSP (#559).

Capturing from chat:
Code generation is already being used for filters and StructArray because it's significantly faster than the alternative. @mourner attempted fast non-eval feature-filter but it was still 5 times slower mapbox/feature-filter#11

@jfirebaugh
Copy link
Contributor

Nice! I'm bullish on the approximation-based approach for zoom interpolation. (I think we should make the logic consistent between -js and -native though.)

I don't think we should embed the filter grammar directly; it's too awkward mixing it with {ref} syntax.

How much typing do we want? For validation purposes and easier implementation in a statically-typed language, it seems like static typing (as much as possible) is good. We of course can't type check feature-dependent references, but we can for example eschew string concatenation to construct color values in favor of built-in methods to construct them from RGBA components.

@anandthakker
Copy link
Contributor Author

How much typing do we want? For validation purposes and easier implementation in a statically-typed language, it seems like static typing (as much as possible) is good. We of course can't type check feature-dependent references...

👍 on typing as much as possible.

..., but we can for example eschew string concatenation to construct color values in favor of built-in methods to construct them from RGBA components

Agreed -- the string concatenation for colors is both fragile and cumbersome. Added color functions to the list (still need to review csscolorparser and our color spaces functionality to get a comprehensive list -- will do)

@anandthakker
Copy link
Contributor Author

I don't think we should embed the filter grammar directly; it's too awkward mixing it with {ref} syntax.

Yeah; I think we should just make expression-function versions of each of the filter operations.

This will be clearer, and also open the door for extending feature filters to allow expression-based filters (though of course we'll still have to keep the existing ones, too, until we're ready for a v9 of the style spec)

@ChrisLoer
Copy link
Contributor

This looks awesome!

On the "what zoom levels should we use" question, we need to keep #4558 in mind. Supporting multiple zoom stops within the [tileZoom, tileZoom + 1] range would make it even more complicated to calculate the single "zoom at which collision occurs" number that we need to pass to the shaders for each label. Doing collision detection at render time would make that problem easier, but even with "future of labeling" (#4704) changes I don't think we have a firm plan to do collision detection synchronously.

@anandthakker
Copy link
Contributor Author

Good point @ChrisLoer -- in light of this and in the interest of making js and native consistent, I updated the proposal to just make it tile.zoom, tile.zoom + 1. I think this is also much easier to document/explain ("Expressions that refer to the map zoom level will only be evaluated at integer zooms. When the map is at non-integer zoom levels, the expression's value will be approximated using linear or exponential interpolation.")

@anandthakker
Copy link
Contributor Author

Moved the expression AST details into expressions.md so that we can comment directly on it & track changes.

Copy link
Contributor

@jfirebaugh jfirebaugh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a bunch of syntax suggestions. Suggest we go full lisp and make everything an s-expression.

# Style Function Expressions

**Constants:**
- LN2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the syntax for constants? Suggestion: ["ln2"], ["pi"], etc.

- JSON string / number / boolean literal

**Property lookup:**
- Feature property: `{ "ref": "feature", "key": propertyName, "default": defaultValue}`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Syntax alternatives:

  • ["data", key, default], where key and default are both expressions
  • ["data", key], where key is an expression, and we also provide a ["has", key] expression, with which arbitrary default logic can be implemented.


**Property lookup:**
- Feature property: `{ "ref": "feature", "key": propertyName, "default": defaultValue}`
- Map property: `{"ref": "map", "key": "zoom"|"pitch"|… }`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Syntax alternative: ["zoom"], ["pitch"], etc.

- Alternative: `[` `"``if``"``, [ filter_expr, expr_if_true ], [ filter_expr, expr_if_true], …, expr_otherwise]` (where the first matching filter wins)

**Boolean:**
- has, !has
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should also have ["not", expression] in addition to, or instead of ! variants of individual operators.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be equivalent to ["none", expression], right?

Copy link
Contributor

@jfirebaugh jfirebaugh May 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, although "not" is usually considered the more fundamental operator, ["none", e⁰ ... eⁿ] being expressed as ["not", ["any", e⁰ ... eⁿ]].

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point... I keep getting stuck thinking in terms of existing filter syntax, but there's no real need to match that.


**String:**
- `["concat", expr1, expr2, …]`
- `["transform", string_expr, "uppercase"|"lowecase"]`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Syntax alternative: ["upcase", expression], ["downcase", expression].

Copy link
Contributor

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s really exciting to see a proposal for this feature. This will solve a lot of longstanding problems that have blocked improvements to our cartography.

In this review, I’m mainly just jotting down notes about how to translate between the syntax proposed here (for the style JSON file format) and NSExpression (in Objective-C and Swift, targeting the iOS and macOS SDKs). mapbox/mapbox-gl-native#8074 proposes to simplify the runtime styling API so that layout and paint properties are set to instances of NSExpression.

NSExpression is desirable for consistency with Cocoa APIs; for consistency with style layer filters, which are exposed as NSPredicate objects and thus share a single format string DSL; and for keeping the runtime styling API concise enough that developers would actually adopt expressions as runtime style values. (See Apple’s “Predicate Programming Guide” for a BNF grammar for this DSL.)

Some of my comments below point out places where syntactic sugar can be expressed by more essential parts of the proposal. In general, I think it would be nice to tightly scope this feature, not only to ensure that style expressions can bridge losslessly to and from NSExpression, but also to avoid adding not-strictly-necessary features now that limit our flexibility later.

**Constants:**
- LN2
- PI
- e
Copy link
Contributor

@1ec5 1ec5 May 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I’m aware, NSExpression comes with no mathematical constants, since the expectation there is that the developer could use an existing constant like M_PI programmatically. But since expressions may also occur in style JSON files, it’s understandable that the proposal includes constants. The iOS/macOS SDK can perhaps define variables like $ln2 for use in format strings. ($ variables are a built-in feature of NSExpression; there’s no potential for overlap with $ properties like $id, which must be specified via the %K format specifier.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think using $ to denote constants will help make the style-spec and our expression grammar more consistent. I could go either way on uppercase/lowercase.The uppercase makes it much easier to parse expressions when looking at a style spec as opposed to working with an SDK (not sure how important that use case is)

Wouldn't it be easier to read and debug if we used "$ln2", "$pi" ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue with introducing a mini-syntax inside of string literals is that you then also have to include an escaping syntax, so that the literal string "$pi", or other literals starting with a dollar sign, remain expressible. If you pick backslash as the escape character, then that has to be compounded with the escaping syntax of JSON itself ("\\$pi"). This is confusing accidental complexity. Better to keep the grammar simple and use s-expressions as the only syntactical element. Expose constants as constant functions: ["pi"].

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that we need to avoid a mini-language in the JSON format – that’s what derailed #4079 for so long.

My original comment above refers to an approach we could take for NSExpression conversion, since that DSL already makes a distinction between key paths, variables, and strings. For example, the iOS SDK has the following guidance for using the special $id and $type feature properties in a predicate:

It is possible to create expressions that contain special characters in the predicate format syntax. This includes the $ in the $id and $type special style attributes and also hyphen-minus and tag:subtag. However, you must use %K in the format string to represent these variables: @"%K == 'LineString'", @"$type".

In that example, the developer would want $type treated as a key path. If they put $type directly in the format string, NSPredicate interprets it as a variable instead. For these mathematical constants, however, the developer would expect it to be interpreted as a variable, making an inline $pi desirable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to keep the grammar simple and use s-expressions as the only syntactical element.

By the way, one could make the same argument for treating bare strings in the JSON as constant values always and requiring the use of a {ref: …} object to represent a property reference. But that would either move the expression syntax away from the filter syntax or necessitate a backwards-incompatible change to the filter syntax.

NSExpression format strings make a distinction between keyPath and 'string' or "string", which makes it possible to represent the style JSON ["==" "property", "string"] as either property = 'string' or 'string' = property in an NSExpression format string.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, one could make the same argument for treating bare strings in the JSON as constant values always and requiring the use of a {ref: …} object to represent a property reference. But that would either move the expression syntax away from the filter syntax or necessitate a backwards-incompatible change to the filter syntax.

See #4715 (comment)

Copy link
Contributor

@jfirebaugh jfirebaugh May 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, one could make the same argument for treating bare strings in the JSON as constant values always and requiring the use of a {ref: …} object to represent a property reference. But that would either move the expression syntax away from the filter syntax or necessitate a backwards-incompatible change to the filter syntax.

Yes, that's exactly what @anandthakker and I are agreeing we should do (except I suggested ["data", …] rather than {ref: …}).

In this proposed grammar, the first element of a JSON array is always a JSON string constant naming a style spec function. The zero or more other elements are expressions. One type of expression is a string constant expression, which is represented by a JSON string constant. In no case does a string constant alone denote "the value of the property with this name". This is indeed a break from the filter syntax.

This does mean that there can't be a direct correspondence between JSON array literals and style array literals. For example, an expression whose result is a font stack array will need to be something like ["array", "Open Sans Regular", "Arial Unicode MS Regular"], not simply ["Open Sans Regular", "Arial Unicode MS Regular"]. (The latter would represent calling the "Open Sans Regular" function -- obviously nonsensical in this particular case, but potentially ambiguous in others.)

- JSON string / number / boolean literal

**Property lookup:**
- Feature property: `{ "ref": "feature", "key": propertyName, "default": defaultValue}`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NSExpression has the concept of key paths but not default values for key paths. So if default is incorporated into the JSON format, the iOS/macOS SDK would lean on NSExpression’s built-in conditional support to rewrite ["data", key, default] as TERNARY(key != nil, key, default).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm coming around to your original suggestion of just relying on the conditional operator to provide default behavior.

{ref: 'feature', key: 'x'},
{ref: 'feature', key: 'y'}
]
]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To illustrate the advantages of using NSExpression in the iOS/macOS SDKs, this paint property would be set using the following line of code (Objective-C then Swift):

circleLayer.circleRadius = [NSExpression expressionWithFormat:@"map.zoom**2 * 0.125 * (20 + feature.x + feature.y)"];
circleLayer.circleRadius = NSExpression(format: "map.zoom**2 * 0.125 * (20 + feature.x + feature.y)")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's so nice! What do map.zoom, feature.x, etc. end up looking like in the resulting NSExpression object?

Copy link
Contributor

@1ec5 1ec5 May 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The initialized NSExpression consists of the following object tree:

  • NSExpression representing the function multiply:by:, the operand _NSPredicateUtilities, and the following NSExpressions as arguments:
    • NSExpression representing the function multiply:by:, the operand _NSPredicateUtilities, and the following NSExpressions as arguments:
      • NSExpression representing the function raise:toPower:, the operand _NSPredicateUtilities, and the following NSExpressions as arguments:
        • NSExpression representing the key path map.zoom (as a string)
        • NSExpression representing the constant value 2
      • NSExpression representing the constant value 0.125
  • etc.

NSExpression comes with a built-in way to evaluate itself given a context dictionary. However, imperative filters aren’t on the roadmap – mapbox/mapbox-gl-native#7860 (comment) – so the iOS/macOS SDK instead translates the AST provided by NSExpression into the AST expected by mbgl.

'circle-color': {
type: 'expression',
expression: [
'concat',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concatenating strings to form a color would be problematic for all the native SDKs, where colors are formed using objects rather than CSS color strings. For example, on iOS, a color-typed property must be set to something that evaluates to a UIColor object, not a string. Setting a constant color might look like this:

circleLayer.circleColor = NSExpression(constantValue: UIColor.orange)

If the JSON file format would instead treat rgb et al. as expression functions in their own right, as opposed to the products of string concatenation, it would be possible for the SDK to define its own custom function based on +[UIColor colorWithRed:green:blue:alpha:]:

circleLayer.circleColor = NSExpression(format: "mgl_colorWithRedGreenBlueAlpha({ 128 + 10 * feature.x, 128 + 10 * feature.y, 128, 1 })")

perhaps with a more memorable convenience method, +[NSExpression expressionWithMGLRedExpression:greenExpression:blueExpression:alphaExpression]:

let redExpression = NSExpression(format: "128 + 10 * feature.x")
let greenExpression = NSExpression(format: "128 + 10 * feature.y")
let blueExpression = NSExpression(constantValue: 128)
circleLayer.circleColor = NSExpression(mglRed: redExpression, green: greenExpression, blue: blueExpression)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 yep, agreed that having color functions rgb, etc. as opposed to using string concatenation. (Ref: #4715 (comment))

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CSS 4 color-modifying function draft might serve as inspiration here: https://drafts.csswg.org/css-color/#modifying-colors

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woah. That's pretty awesome, although including "color adjuster" functions would probably require taking on a dependency like https://github.com/gka/chroma.js/.

For the scope of this PR, I'm inclined to stick to color constructors, and to ticket/discuss separately the possibility of adding more sophisticated color operations.


**Decision:**
- `["if", boolean_expr, expr_if_true, expr_if_false]`
- Alternative: `[` `"``if``"``, [ filter_expr, expr_if_true ], [ filter_expr, expr_if_true], …, expr_otherwise]` (where the first matching filter wins)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This alternative pattern matching syntax (to use Swift terminology) has no natural equivalent in NSExpression, but the SDK could translate it to nested conditionals:

TERNARY(filter_expr, expr_if_true, TERNARY(filter_expr, expr_if_true, …))

Copy link
Contributor Author

@anandthakker anandthakker May 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As with default values, I'm leaning back towards the first proposal here (the one that's equivalent to ternary operator). Compactness in the JSON representation doesn't seem particularly important, since no one will actually be authoring expressions via AST (except for us when we write unit tests 😄 )

**Boolean:**
- has, !has
- ==, !=
- >, <, >=, <=, between(?),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NSExpression and NSPredicate have a BETWEEN operator that’s inclusive on both ends. The iOS and macOS SDKs currently rewrite (key >= min && key <= max) as key BETWEEN { min, max }: mapbox/mapbox-gl-native#6405 mapbox/mapbox-gl-native#7548.

**Numeric:**
- +, -, \*, /, %, ^ (e.g. `["+", expr1, expr2, expr3, …]`, `["-", expr1, expr2 ]`, etc.)
- log10, ln, log2
- sin, cos, tan, asin, acos, atan
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These trigonometric functions aren’t listed as being built into NSExpression, but we could define our own functions.

@mb12
Copy link

mb12 commented May 18, 2017

@anandthakker Is it reasonable to expect that any new operators, functions, expressions will work seamlessly with filters as well? The language used to select "list of features to layout" should at least be as expressive as language used to describe style functions (with limitations).

@anandthakker
Copy link
Contributor Author

Is it reasonable to expect that any new operators, functions, expressions will work seamlessly with filters as well?

Yes. We don't want to make a breaking change to the existing filter syntax, so my current thinking is that we'll extend the filter spec to allow both "legacy" filters and the new boolean expression functions. Not sure yet whether we'll need to add any extra wrapping to avoid ambiguity between the two, but we'll cross that bridge when we come to it

- Feature property: `[ "data", key_expr ]`
- Map property:
- `[ "zoom" ]` (Note: expressions that refer to the map zoom level are only evaluated at integer zoomslevels. When the map is at non-integer zoom levels, the expression's value will be approximated using linear or exponential interpolation.)
- `[ "pitch" ]`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would feature property lookups require a data function while zoom and pitch be usable independently of a map function, similar to constants? Is it because the set of map properties is finite and well-defined, in contrast to the set of potential feature properties?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it because the set of map properties is finite and well-defined, in contrast to the set of potential feature properties?

^ yeah, that's my thinking. there's no need for it to be [ "map", "zoom" ] when we can simply reserve [ "zoom" ] as a 0-ary function.

- in, !in
- all, any, none
- `[ "has", key_expr ]`
- `[ "==", expr1, expr2]`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would the style author specify that a comparison between two strings should be performed case- or diacritic-insensitively (#4136)? Or would we require downcase and a strip-diacritics function to be applied explicitly to either operand?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or would we require downcase and a strip-diacritics function to be applied explicitly to either operand?

This approach sounds good to me...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or would we require downcase and a strip-diacritics function to be applied explicitly to either operand?

I’m not so sure that would be robust enough for internationalization purposes: #4136 (comment).

Relatedly, should == take multiple arguments in order to say, for instance, a == b == c? (If not, that would open the door to using the second argument for folding flags.) If == shouldn’t take multiple arguments, should arithmetic operators like * take multiple arguments?

math: true
},
'min': {
input: [Type.Array[Type.Number]],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, Type.Array[t] really just means 'n arguments of type t'; given that we'll probably end up needing to use that for actual array values (like ["array", e0, e1, e2...]), this should probably be changed to something like Type.Nargs[t].

@samanpwbb
Copy link
Contributor

samanpwbb commented May 18, 2017

How will arbitrary functions relate to existing function syntax? I can see how most functionality in current function syntax could be replicated with arbitrary functions (a zoom + arithmetic operation and maybe an if statement or two for more complex cases). I can also see ways that the two types of functions could be combined.

I strongly recommend we strive for a spec that promotes a single way to do things (for example, either remove camera expressions from arbitrary functions and allow arbitrary functions as return values in zoom functions, or keep camera expressions in arbitrary functions and have a real plan in place to deprecate zoom functions).

@1ec5 1ec5 added the cross-platform 📺 Requires coordination with Mapbox GL Native (style specification, rendering tests, etc.) label May 18, 2017
@anandthakker
Copy link
Contributor Author

keep camera expressions in arbitrary functions and have a real plan in place to deprecate zoom functions).

This is my strong preference: we'll still have to support stop-based functions for a while yet in GL implementations, but once we get arbitrary expressions in place, I propose that we formally mark stop functions as deprecated and exclusively focus on arbitrary expressions in docs/examples, Studio, and any other tools.

@ansis
Copy link
Contributor

ansis commented May 19, 2017

Overall the direction of this stuff sounds great!

I have some concerns about supporting map properties in expressions.

implementation

How will the approximate expressions that use pitch or rotation be implemented? What if they use pitch, zoom and rotation?

interpolation

The plan for implementing expressions that use zoom is to evaluate them at several zoom levels and interpolate them. Will they be interpolated linearly? exponentially? it seems like for either [‘*’, [‘zoom’], 4] or [‘^’, 2, [‘zoom’] the interpolation within a zoom level won’t be what you expect.

How would you write an expression for a line-width that is:

  • 2px at zoom 10
  • 12px at zoom 18
  • increases at some kind of exponentialish curve in between?

approximation

The approximation used by the implementation would have to be part of the style spec, right? At zoom 4.5 would [‘concat’, ‘zoom ’, [zoom]] evaluate to zoom 4?

In previous style function discussions it seemed important to support changes to functions at fractional zoom levels. Is this still important to support? Would [if [‘>’, [‘zoom’], 4.5], 10, 0] result in a value that smoothly increases between z4 and z5 or the expected sharp jump?

UI

I'm not caught up on the plans for dds in studio, but:

Expressions are more flexible, and with this flexibility comes the increase in variation and dimensionality that makes it harder to make UI's for.

A goal with the property+zoom functions was to make the form regular. They are basically a table. On one axis you have zoom on the other axis you have properties. I'm thinking a free form expression is going to a lot hard to build a ui for?

I strongly recommend we strive for a spec that promotes a single way to do things (for example, either remove camera expressions from arbitrary functions and allow arbitrary functions as return values in zoom functions, or keep camera expressions in arbitrary functions and have a real plan in place to deprecate zoom functions).

I agree. I'm thinking we should leave exist functions responsible for zoom allow expressions wherever we allow property values.

@ansis
Copy link
Contributor

ansis commented May 25, 2017

A bunch of stuff that might be worth supporting:

scales

  • a way to map exact values to subexpressions, replacing type: categorical
  • a way to match a value to a corresponding interval, replacing type: interval
  • a way to easily interpolate values exponentially, replacing type: exponential
  • a way to easily interpolate values linearly

external prior art: https://github.com/d3/d3-scale

formatting, localization

  • number to string with specified number of decimal places (1.2345 to "1.23")
  • number to localized string (21000 to "21,000" or "21 000" depending on locale)
  • methods for converting between distance units (metres, feet, kilometres, miles)
  • some way of localizing text by choosing the right name property
    • maybe ["localize", lambda] where lambda is a user-supplied function that returns text or undefined for a specific language code. ["localize", ...] would use the map's/browser's/device's language(s) and some fallback algorithm to get the best text that is available
  • lambdas, to support ^
  • time formatting/localization? temperature? money?

@anandthakker
Copy link
Contributor Author

@ansis interesting re: localization. Rather than introducing lambdas, would it suffice to add one or more device-driven constants (['locale'] or something?), which could be used in ifs or switches?

@anandthakker
Copy link
Contributor Author

  • Updated top of ticket to reflect the most recent consensus, and w/ a few open items.
  • Updated expressions.md with a rough draft of copy for the style-spec docs reflecting same

@1ec5
Copy link
Contributor

1ec5 commented May 25, 2017

There was previously a proposal in mapbox/mapbox-gl-native#7860 to allow the use of lambdas as layout or paint property values. Lambdas (blocks in Objective-C parlance, closures in Swift parlance) would be very attractive to developers and technically feasible on the native side, but there’s a risk that developers would tend to introduce concurrency bugs with such an API. On iOS, blocks are used quite commonly with NSExpression, but normally in the context of database querying, where a developer might anticipate having to be careful about thread safety.

@ansis
Copy link
Contributor

ansis commented May 25, 2017

@ansis interesting re: localization. Rather than introducing lambdas, would it suffice to add one or more device-driven constants (['locale'] or something?), which could be used in ifs or switches?

My thinking is that the fallback logic can be pretty complex and isn't something you want people to have to provide themselves every time they add text. I don't know much about localization so this definitely needs further research. I know a bunch of other people at mapbox have thought a lot about this stuff.

**Comparison and boolean operations:**
- `[ "==", expr1, expr2]` (similar for `!=`)
- `[ ">", lhs_expr, rhs_expr ]` (similar for <, >=, <=)
- `[ "between", "[]"|"(]"|"[)"|"()", test_expr, left_boundary_expr, right_boundary_expr ]` - tests whether `test_expr` is "between" the left and right boundaries, with the first argument dictating whether the each boundary is exclusive or inclusive.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is [) / (] / etc. argument just too weird? AFAICT there's no clear choice that's more common/expected than all the others...

Copy link
Contributor

@1ec5 1ec5 May 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to provide multiple kinds of between, given that between is syntactic sugar anyways? I realize there’s a tension between [) being the most programming-friendly option and [] being the option that iOS/macOS would naturally expose, but I figured iOS/macOS would synthesize BETWEEN from < && >, without the need for a dedicated between operator in the style specification, just as it does for filters today.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I've gone back and forth on this. I had removed it, but then reinstated it while thinking about how to express interval type property functions that stays a bit closer to the intent than breaking it all the way down into basic comparisons would. But I'm coming around to thinking we just include an interval expression type, in which case maybe we just drop between.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Syntax proposal for intervals:

["intervals", x, y1, n1, ..., ym, nm, ym+1]

In this formulation, the input x expression is followed by m+1 possible output y expressions interspersed with m breakpoints. In order to permit efficient evaluation, the breakpoints n1 < ... < nm are required to be numeric literals in strictly ascending order; expressions are not allowed. The result is the first ym such that nm >= x, or ym+1 if x is greater than all n's.

Example:

"icon-image": {
    "type": "expression",
    "value": ["intervals", ["data", "population"],
        "small-dot", 10000, "medium-dot", 100000, "large-dot"]
}

@1ec5
Copy link
Contributor

1ec5 commented May 25, 2017

number formatting

For number formatting (#4119), I think we should consider a function that takes a locale identifier as an argument rather than one that takes a format. (The locale identifier can come from a source or default to the user’s locale.) This appears to be feasible using existing platform APIs (JavaScript, Objective-C, Java).

If on the other hand we allow the developer to specify the format explicitly, it would be more difficult to correctly format Arabic–Indic or Hindu numerals, or to group digits by lakhs and crores. There is a Unicode standard for number format patterns, but I think composing these patterns would add an unnecessary burden on style authors.

@anandthakker
Copy link
Contributor Author

For number formatting (#4119), I think we should consider a function that takes a locale identifier as an argument rather than one that takes a format

I agree, but I think we should handle this in a follow-up to the initial expressions push.

anandthakker pushed a commit that referenced this pull request May 25, 2017
anandthakker pushed a commit that referenced this pull request May 25, 2017
@anandthakker
Copy link
Contributor Author

Continued => #4754

@samanpwbb
Copy link
Contributor

samanpwbb commented Jun 2, 2017

Would it be possible to see practical example expressions for each of the Important use cases that the final design should cover? I would love to see the example expression plus a description of the problem each expression solves for a cartographer.

anandthakker added a commit that referenced this pull request Jun 2, 2017
* Add draft proposal for property expressions spec

Developed in #4715

* Update spec proposal

 * Introduce `curve` expression, use it to cover zoom function `stops`.
 * Introduce object and array literals
 * Remove explicitly typed property lookup

* Describe type system

* Remove ["else"]

* Minor edits
@jfirebaugh jfirebaugh deleted the expressions branch June 14, 2017 21:37
anandthakker added a commit that referenced this pull request Jun 28, 2017
* Add draft proposal for property expressions spec

Developed in #4715

* Update spec proposal

 * Introduce `curve` expression, use it to cover zoom function `stops`.
 * Introduce object and array literals
 * Remove explicitly typed property lookup

* Describe type system

* Remove ["else"]

* Minor edits
anandthakker added a commit that referenced this pull request Jun 30, 2017
* Add draft proposal for property expressions spec

Developed in #4715

* Update spec proposal

 * Introduce `curve` expression, use it to cover zoom function `stops`.
 * Introduce object and array literals
 * Remove explicitly typed property lookup

* Describe type system

* Remove ["else"]

* Minor edits
anandthakker added a commit that referenced this pull request Jul 24, 2017
* Add draft proposal for property expressions spec

Developed in #4715

* Update spec proposal

 * Introduce `curve` expression, use it to cover zoom function `stops`.
 * Introduce object and array literals
 * Remove explicitly typed property lookup

* Describe type system

* Remove ["else"]

* Minor edits
anandthakker added a commit that referenced this pull request Aug 3, 2017
* Add draft proposal for property expressions spec

Developed in #4715

* Update spec proposal

 * Introduce `curve` expression, use it to cover zoom function `stops`.
 * Introduce object and array literals
 * Remove explicitly typed property lookup

* Describe type system

* Remove ["else"]

* Minor edits
anandthakker added a commit that referenced this pull request Aug 4, 2017
* Add draft proposal for property expressions spec

Developed in #4715

* Update spec proposal

 * Introduce `curve` expression, use it to cover zoom function `stops`.
 * Introduce object and array literals
 * Remove explicitly typed property lookup

* Describe type system

* Remove ["else"]

* Minor edits
anandthakker added a commit that referenced this pull request Aug 10, 2017
* Add draft proposal for property expressions spec

Developed in #4715

* Update spec proposal

 * Introduce `curve` expression, use it to cover zoom function `stops`.
 * Introduce object and array literals
 * Remove explicitly typed property lookup

* Describe type system

* Remove ["else"]

* Minor edits
anandthakker added a commit that referenced this pull request Aug 18, 2017
* Add draft proposal for property expressions spec

Developed in #4715

* Update spec proposal

 * Introduce `curve` expression, use it to cover zoom function `stops`.
 * Introduce object and array literals
 * Remove explicitly typed property lookup

* Describe type system

* Remove ["else"]

* Minor edits
anandthakker added a commit that referenced this pull request Aug 29, 2017
* Add draft proposal for property expressions spec

Developed in #4715

* Update spec proposal

 * Introduce `curve` expression, use it to cover zoom function `stops`.
 * Introduce object and array literals
 * Remove explicitly typed property lookup

* Describe type system

* Remove ["else"]

* Minor edits
anandthakker added a commit that referenced this pull request Aug 30, 2017
* Add draft proposal for property expressions spec

Developed in #4715

* Update spec proposal

 * Introduce `curve` expression, use it to cover zoom function `stops`.
 * Introduce object and array literals
 * Remove explicitly typed property lookup

* Describe type system

* Remove ["else"]

* Minor edits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cross-platform 📺 Requires coordination with Mapbox GL Native (style specification, rendering tests, etc.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.